-
Notifications
You must be signed in to change notification settings - Fork 145
Add method to derive x25519 from an ed25519 private key #112
Add method to derive x25519 from an ed25519 private key #112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested this with indy to verify compatibility?
Yes, as much as I can. The last test is deriving a common Indy keypair. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you create a branch in indy-sdk in your own fork with tests showing this working?
@mikelodder7 Tested here: https://github.com/andrewwhitehead/indy-sdk/tree/test/derive-key In particular |
@andrewwhitehead I'm not seeing that function in that branch in that file. Can you provide the direct link? |
@mikelodder7 It's over in https://github.com/andrewwhitehead/indy-sdk/blob/test/derive-key/libindy/tests/crypto.rs The whole test suite runs anyway, apart from some unrelated pool genesis transaction tests. |
The function that does the bilateral mapping by dalek-X25519 can be found here: https://github.com/dalek-cryptography/curve25519-dalek/blob/17698df9d4c834204f83a3574143abacb4fc81a5/src/edwards.rs#L473 On Indy-sdk side I've tracked it down to using this libsodium function: https://github.com/jedisct1/libsodium/blob/927dfe8e2eaa86160d3ba12a7e3258fbc322909c/src/libsodium/crypto_sign/ed25519/ref10/keypair.c#L46 They appear to share the same math function of (1+Y_P)/(1-Y_P) although the dalek implementation seems to be doing a different equation of (Z+Y)/(Z-Y) which depends on "Z" that's considered equivalent in the comments. So from what I can tell they're both doing this mapping of points, but using different math operations to do so is all. |
@andrewwhitehead I see the test but what I'm interested in is whether this code you are pushing to Ursa is functionally the same. I actually think this code could be moved to Indy and kept out of Ursa since this is an Indy specific implementation. |
This is quite common functionality in libraries. From what I know, libsodium, monocipher, and dalek-cryptography currently support this. I see it as beneficial to be kept within ursa. |
I think the I don't think |
@andrewwhitehead and I met over zoom to discuss the details. I'm happy with it now. |
@@ -34,6 +34,36 @@ impl Ed25519Sha512 { | |||
))), | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comments here to document the purpose of the sign_key_to_key_exchange function and how to use it.
let secret = x25519_dalek::StaticSecret::from(output); | ||
Ok(PrivateKey(secret.to_bytes().to_vec())) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comments here to document the purpose of the expand_keypair function and how to use it
…ey creation Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
ccf30b5
to
6d8904f
Compare
This method is compatible with libsodium's
crypto_sign_ed25519_sk_to_curve25519
.This PR also adds
Ed25519Sha519::keypair_from_secret
which is compatible with libsodium'scrypto_sign_seed_keypair
. @mikelodder7 I didn't add this as a separate KeyGenOption as it didn't seem to be generally applicable, but I'll let you be the judge.